DSP-24890 Adds OIDC support #20
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
Adds OpenID Connect (OIDC) authentication support when connecting to DSE/HCD databases.
- Upgrades project Java version from 8 to 11 and removes deprecated Animal Sniffer plugin blocks.
- Introduces new OIDC configuration options and wiring in both the config class and lifecycle manager.
- Adds the
db-advanced-auth-client-plugindependency to all relevant modules.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| text/pom.xml | Removed Animal Sniffer plugin configuration |
| pom.xml | Bumped Java version to 11; added new auth plugin |
| common/src/.../LifeCycleManager.java | Added OIDC provider branch to processAuthenticatorConfig |
| common/src/.../AuthenticatorConfig.java | Defined new OIDC config options and validation |
| common/pom.xml | Declared db-advanced-auth-client-plugin dependency |
Comments suppressed due to low confidence (3)
common/src/main/java/com/datastax/oss/common/sink/config/AuthenticatorConfig.java:97
- The new OIDC options are not yet documented in the project's README or user guide. It would help users if you add examples and descriptions for
auth.oidc.*settings in external documentation.
"OIDC Authentication server url")
common/src/main/java/com/datastax/oss/common/sink/state/LifeCycleManager.java:625
- The new OIDC code path isn't covered by existing unit or integration tests. Please add tests for
OIDCClientCredentialsAuthProviderconfiguration and error scenarios.
} else if (authConfig.getProvider() == AuthenticatorConfig.Provider.OIDC) {
common/src/main/java/com/datastax/oss/common/sink/config/AuthenticatorConfig.java:44
- It looks like
SERVICE_OPTwas accidentally renamed toauth.gssapi.cservice, which will break existing GSSAPI configurations. Please revert it toauth.gssapi.service.
public static final String SERVICE_OPT = "auth.gssapi.cservice";
| @@ -69,6 +69,10 @@ | |||
| </exclusion> | |||
| </exclusions> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>com.datastax.db</groupId> | |||
| <artifactId>db-advanced-auth-client-plugin</artifactId> | |||
There was a problem hiding this comment.
The new db-advanced-auth-client-plugin dependency is missing a <version> declaration, which may cause build failures. Please add the appropriate ${datastax.advanced-auth-client-plugin.version} or explicit version.
Suggested change
| <artifactId>db-advanced-auth-client-plugin</artifactId> | |
| <artifactId>db-advanced-auth-client-plugin</artifactId> | |
| <version>${datastax.advanced-auth-client-plugin.version}</version> |
Copilot uses AI. Check for mistakes.
Comment on lines
+289
to
+296
| public URI getOIDCIssuer() { | ||
| try { | ||
| return new URI(getString(OIDC_ISSUER_OPT)); | ||
| } catch (URISyntaxException e) { | ||
| // This should never happen because we validate the URI in the constructor. | ||
| return null; |
There was a problem hiding this comment.
[nitpick] Returning null from getOIDCIssuer() can lead to NPEs downstream. Consider throwing an unchecked exception or using Optional<URI> to clearly signal failure.
Suggested change
| public URI getOIDCIssuer() { | |
| try { | |
| return new URI(getString(OIDC_ISSUER_OPT)); | |
| } catch (URISyntaxException e) { | |
| // This should never happen because we validate the URI in the constructor. | |
| return null; | |
| public Optional<URI> getOIDCIssuer() { | |
| try { | |
| return Optional.of(new URI(getString(OIDC_ISSUER_OPT))); | |
| } catch (URISyntaxException e) { | |
| // This should never happen because we validate the URI in the constructor. | |
| return Optional.empty(); |
Copilot uses AI. Check for mistakes.
common/src/main/java/com/datastax/oss/common/sink/state/LifeCycleManager.java
Outdated
Show resolved
Hide resolved
Adds OIDC configuration options to connect to DSE/HCD. This patch enables the following settings in sink-config.json file: * setting "auth.provider": "OIDC" enabling the usage of Java Driver Plugin auth provider implement Client Credentials Flow * setting "auth.oidc.*" settings with required parameters such as "issuer", "client_id", "client_secret", etc. Logic to enable the OIDC auth provider will be done in the following commit.
Adds DataStax Advanced Auth Client Plugin with OIDC support. Updates project to JDK 11 (currently minimum requirement for the Auth Client Plugin) and removes checks for JDK 8.
Updates Jenkins runner as the bionic image is no longer configured causing the job to get stuck.
Sets JDK 11 as the default java version for building and running tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds OIDC support when connecting to DSE/HCD databases.